-
Notifications
You must be signed in to change notification settings - Fork 892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(#3759): Implements Apple Pencil double tap functionality #3768
base: master
Are you sure you want to change the base?
feat(#3759): Implements Apple Pencil double tap functionality #3768
Conversation
…nality Implements rust-windowing#3759 Also see rust-windowing#99 Based on the `master` branch
The windows nightly checks are failing for a completely unrelated reason |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be supported on MacOS as well?
See #99 (comment) as well. Though I'm not entirely against just going ahead here, I would prefer if we implement barrel and eraser first and plan ahead a bit so we don't introduce another completely platform-specific event before moving WindowEvent
to traits and allowing platform-specific extensions.
@@ -350,6 +392,29 @@ impl WinitView { | |||
this.setContentScaleFactor(scale_factor as _); | |||
} | |||
|
|||
// adds apple pencil support | |||
// let view: Retained<UIView> = self.view().expect("View has already loaded"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is leftover, because the apple docs recommend retrieving the view like the commented code
// let view: Retained<UIView> = self.view().expect("View has already loaded"); | ||
let interaction: Retained<UIPencilInteraction> = { | ||
let allocated: Allocated<UIPencilInteraction> = | ||
MainThreadMarker::new().unwrap().alloc(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't you using the MainThreadMarker
passed into this function by mtm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch thanks
since this enum techniquely covers more than just taps when squeezes are added
You can't connect Apple Pencils to MacOS, even using Mac Catalyst. If you can I don't know about it and would love to play around!
Hold on a minute, you can define platform specific events? That fits this PR way better than adding the current API, since this PR's API must be platform-independent but has only one implementation! About barrel and eraser, I don't know what those are and doubt I have the hardware for it anyway since I only have an Apple Pencil Gen 2. I don't really want to implement too much more than this since it is my original use case and all I can physically test at the moment, but I could certainly add it if it will help this functionality get merged. By barrel and eraser your referring to windows stylus pens (among others)? |
Its possible through Apple Sidecar or third party alternatives like Duet Display and Luna Display.
Unfortunately not, I was speaking about future changes to the API we have planned, where we could merge something like this without much of a second thought.
I think there is a misunderstanding. My proposal is to implement a basic pen API that follows the Web API spec before implementing platform specific functionality so we know how and where to fit it in the API.
Indeed, which is why its platform specific. I'm planning to make a PR that implements the basic pen API for Web today. So hopefully we will have a clearer picture afterwards. |
Implements #3759
Also see #99
Based on the
master
branchchangelog
module if knowledge of this change could be valuable to usersPorted from #3763, relevant sections pasted below:
Also, I have renamed the API name from
PenEvent
->PenSpecialEvent
(can refactor toSpecialPenEvent
I don't mind) to disambiguate these events from the generic pen events as discussed in #99To re-iterate why I don't think these can be implemented as part of the Web API in #99, these sorts of events are not associated with a screen tap or specific touch gesture. I want the ability to handle these events either way they are implemented however, so happy to help in any way to get this functionality into a release usable from
bevy